Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HandleFuncs can now be Type Methods as well. #2

Closed
wants to merge 2 commits into from

Conversation

ivaningrooves
Copy link

http.HandleFunc documentation was not discovered because only
first class functions were traversed when building apib documentation.

Now both first class functions and type methods are traversed.
For example:

type WidgetController struct {}

// GetTypeWidget retrieves a single Widget
func (* WidgetController) GetWidget(w, r) { }

`http.HandleFunc` documentation was not discovered because only
first class functions were traversed when building apib documentation.

Now both first class functions and type methods are traversed.
For example:

		type WidgetController struct {}

    // GetTypeWidget retrieves a single Widget
		func (* WidgetController) GetWidget(w, r) { }
@s-mang
Copy link
Owner

s-mang commented Jun 23, 2016

Hi! Thanks so much for the PR, I'm sorry I haven't had a chance to review your code yet, things have been busy with work and Women Who Go. I will try to review your code this weekend.

Sarah

@ivaningrooves
Copy link
Author

Hi Sarah,

funny when you said Women Who Go at first, I thought you meant the go board
game. After googling your organization, I am very impressed with what you
are doing! I am using your library for a work project, I work with
INgrooves music in Victoria B.C. Canada. It seems my PR's tests are failing
at the moment. Let me look into this as soon as I have more time.

Cheers, Ivan

Ivan

On Wed, Jun 22, 2016 at 5:55 PM, Sarah Adams [email protected]
wrote:

Hi! Thanks so much for the PR, I'm sorry I haven't had a chance to review
your code yet, things have been busy with work and Women Who Go. I will try
to review your code this weekend.

Sarah


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/APx-82uVfbjB4MIZrWkp1uqyExO5jM2eks5qOdmRgaJpZM4I2yaK
.

@s-mang
Copy link
Owner

s-mang commented Jul 18, 2016

Yea, the tests were already broken. You can run the example pkg tests and check the output manually.

You've rewritten all the imports to github.com/ivaningrooves/test2doc instead of github.com/adams-sarah/test2doc so there's no way I can merge this ATM.

@ivaningrooves
Copy link
Author

Yes, how would you do this if you were me? The PR contains 2 commits; the
first one is the actual change, the second involves the import rewrite so I
was able to confirm it works locally.

Ivan

On Mon, Jul 18, 2016 at 9:57 AM, Sarah Adams [email protected]
wrote:

Yea, the tests were already broken. You can run the example pkg tests and
check the output manually.

You've rewritten all the imports to github.com/ivaningrooves/test2doc
instead of github.com/adams-sarah/test2doc so there's no way I can merge
this ATM.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/APx-8_WlDSgAntfP_A3vCrP9oGg9aTFgks5qW7BwgaJpZM4I2yaK
.

@s-mang
Copy link
Owner

s-mang commented Jul 18, 2016

Ah, so pull requests are based off branches. So you need to get rid of the second commit in this branch.

What I would do is:

$ cd $GOPATH/github.com/ivaningrooves/test2doc
$ git checkout master # you're probably already on the master branch
$ git checkout -b tmp # save the current state of your master branch on a new branch, `tmp`
$ git checkout master # checkout master branch again
$ git reset --hard HEAD^ # reset backwards 1 commit from HEAD
$ git push -f origin master # force-push the new state of your master branch

Now if you ever want to look at the second commit again, it will be on the tmp branch locally.

Hope that helps!

@ivaningrooves
Copy link
Author

Thank you!

You should be able to build & run the PR now.

Ivan

On Mon, Jul 18, 2016 at 10:48 AM, Sarah Adams [email protected]
wrote:

Ah, so pull requests are based off branches. So you need to get rid of the
second commit in this branch.

What I would do is:

$ cd $GOPATH/github.com/ivaningrooves/test2doc
$ git checkout master # you're probably already on the master branch
$ git checkout -b tmp # save the current state of your master branch on a new branch, tmp
$ git checkout master # checkout master branch again
$ git reset --hard HEAD^ # reset backwards 1 commit from HEAD
$ git push -f origin master # force-push the new state of your master branch

Now if you ever want to look at the second commit again, it will be on the
tmp branch locally.

Hope that helps!


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/APx-8_ORsd0KggMpi2kMYxk7bmuA_OSsks5qW7xhgaJpZM4I2yaK
.

@s-mang
Copy link
Owner

s-mang commented Jul 18, 2016

Hm, it looks like the imports are still pointing at your code and not mine.

Did you follow the steps I outlined above? Or did you do something else

./scripts/combine.sh
```

final apib doc file is generated at `example/apidoc.apib`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why has this file been deleted?

@s-mang
Copy link
Owner

s-mang commented Jul 18, 2016

It looks like you deleted my example pkg too :(

@ivaningrooves
Copy link
Author

Hi Sarah,

I did remove the changes from the PR but I had to add them back to my fork
to not break my companie's CI pipeline. I don't understand why my second
revision gets added to the PR automatically. I might have to create a new
PR... Give me moment to fix it.

Ivan

On Mon, Jul 18, 2016 at 4:28 PM, Sarah Adams [email protected]
wrote:

It looks like you deleted my example pkg too :(


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/APx-85xaqHqGfQurZL7N0xdO8w95_Mgqks5qXAwegaJpZM4I2yaK
.

@ivaningrooves
Copy link
Author

I created a new PR #3

@ivaningrooves
Copy link
Author

See #3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants